Land Looking-Around users in Spend > Expenses and add "Send to someone" to a tracked expense's More menu - #97883
Land Looking-Around users in Spend > Expenses and add "Send to someone" to a tracked expense's More menu#97883MelvinBot wants to merge 16 commits into
Conversation
… row Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
🚧 shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
This comment has been minimized.
This comment has been minimized.
|
I looked into the failing checks — there's nothing to fix in the code. Every code/test check passes (typecheck, all 8
Since I couldn't run No code changes were pushed because none are needed — the branch is green on everything a code change could affect. |
|
Both failing checks are human/process gates, not code failures — there's nothing to fix with a push.
Every actual code/CI check is green: |
|
@MelvinBot when a new user signs up and selects "Something else" we should still drop them on the Home page, not the Spend page. Then when they make an expense from the Home page, we should drop them into the Spend page. |
|
@MelvinBot for the "Send to someone" menu item - can we place it at the top of the popover menu and put a divider under it?
|
… expense create Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Updated the Bug 1 approach per your feedback in What changed:
So the flow is now: sign up → "Something else" → land on Home → create an expense → land on Spend. Scope is narrow: this only changes routing for Verification: |
|
Re-checked after the latest commit
Passing on this commit: |
Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
I re-checked, and the two failing checks are unchanged — both are human/process gates that a code push can't clear, so there's no fix to push:
Every code/CI check is green ( |
|
Done — moved "Send to someone" to the top of the More popover with a divider beneath it in It now sits in its own top section in Verified on web (created a self-tracked expense, opened Spend › View Expense → More): More popover — "Submit it to someone" first, divider beneath |
|
🚧 shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
|
Rechecked after the latest commit ( The only two remaining red checks are human/process gates that no push can resolve:
So there's no code fix to make here — the PR just needs the author checklist completed and an independent approval to go fully green. |
This comment has been minimized.
This comment has been minimized.
|
Still the same two failures, and both are human/process gates — there's nothing to fix with a code push:
Every actual code/CI check is green ( |
|
@MelvinBot just tested and a new user who selects "Something else" is still getting directed to the Spend page. Can you confirm your changes made it into the adhoc, or if you made that change at all? |
|
Yes — I made the change, and it's in the latest adhoc but not the first one. It comes down to which build you tested:
I also verified at the code level that no other path routes a freshly-onboarded "Something else" user to the Spend/Search tab — that branch only fires on expense creation now ( One heads-up on terminology: |
|
🚧 shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@MelvinBot new user who signs up is still getting dropped into the Spend page. They need to be dropped onto the Home page if they select "Something else". Can you please fix? |
|
I traced this end-to-end and the latest commit on this branch ( What the current code does
I confirmed there's no default-route or "empty inbox → Search" guard that would override this; every signup/init path resolves to Home. So why are you still seeing Spend? Most likely one of:
One honest caveat: I couldn't reproduce the fresh-signup onboarding flow live in this run — the automated test session is an already-onboarded account, and this path needs a brand-new "Something else" signup. My conclusion above is from a full static trace of the code, not a live run. If you re-test the latest build, don't create an expense, and it still drops you on Spend, please grab a quick screen recording — that would mean there's a runtime path the code trace didn't surface, and I'll dig into that exact scenario. |
|
🚧 shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@MelvinBot the flow still isn't correct. Here is the desired flow:
Can you please implement that? |
…nboarding Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
Follow-up review + PR size breakdown (next comment)1. Review items from the previous pass
Test count went from 4 to 12, and 2. Remaining issues🟠 Six of eight Scoping the gate to // ScanSkipConfirmation.tsx:256 and :330
// handleMoneyRequestStepDistanceNavigation.ts:345 and :426
// IOUAmountSubmission.ts:426
isLookingAroundUser, // ...and no isSelfDMDestination
const isUserOnInbox = isReportTopmostSplitNavigator() && !(true && false); // -> unchangedBug this causes: Bug 1 is only actually fixed for the confirmation-step path. A brand-new LOOKING_AROUND user who takes a photo and skips confirmation, or enters an amount via QAB, or logs a distance and skips, still gets dumped into Personal Space. That is the single most common way a new user creates their first expense, so the headline bug is unfixed on the path that matters most.
Two of the six are dead by construction rather than by omission: Fix: either wire 🟠 function getSkipConfirmationPreMountDestinationRoute(shouldSkipConfirmation: boolean, reportID: string | undefined): Route | undefined {
if (!shouldSkipConfirmation || isSearchTopmostFullScreenRoute() || !reportID) {
return undefined;
}
return ROUTES.REPORT_WITH_ID.getRoute(reportID);
}If the skip call sites above get 🟡 submit2026 divergence on the new row The component now reads 🟢 New row skips the write-action guard its neighbour uses. 🟢 Tests / Offline tests / QA Steps are still the empty template, and the PR is no longer a draft. Given the concerns being raised about this PR, this is the cheapest thing to fix and probably the biggest driver of the "not going well" perception. |
3. Size breakdown: what is actually causing the diff sizeCurrent diff: 380 added / 8 removed across 32 files. Split by concern:
My intuition on Slack was that the bulk came from "Send to someone" being a new feature. The data says the opposite. The navigation fix is roughly 2x the size of the new feature, and that ratio holds even if you strip tests out (112 vs 92 production lines). Why Bug 1 is big, and why that is not the author's doing. Post-create routing in this codebase is decided by three functions that must agree with each other:
If you change one and not the others, you get a visible flash or a stale route. So any change to "where does a user land after creating an expense" is a minimum of three coordinated edits plus every call site that feeds them. Of Bug 1's 112 production lines, only about 40 are actual decision logic. The other ~72 are threading one boolean through 8 call sites and 2 param types, at 2 to 9 lines each. That fan-out is pre-existing architecture. Nobody, under any author, does this in fewer files. Why Bug 2 is smaller than it sounds. "Send to someone" reuses What is genuinely removable right now: about 19 lines across 5 files (the dead call sites in section 2), which is ~5% of the diff. That is real, and it should go. It is not "much bigger than needed." 4. VerdictThe concern that this PR grew beyond the issue is fair on its face, but the cause is being misattributed. It is not the new feature and it is not the author. It is that the issue bundles a 1-line-looking behaviour change that sits behind a 3-function navigation contract with a genuinely new UI surface. Either one alone would be a normal-sized PR. Together they read as sprawl. Reassigning the PR to a different author changes the diff by zero lines. Both halves are structurally required regardless of who writes them. If we want a smaller PR, the lever is scope, not authorship. 5. Options, graded by resulting PR sizeA. Melvin finishes this PR as-is, both bugs (recommended)
B. Melvin finishes, but Bug 1 is scoped to the confirmation step
C. Split into two PRs, same author
D. Reassign the PR to a different author
My recommendation is A. If the goal is specifically a smaller diff rather than a finished fix, C is the honest way to get there; D does not get there at all. cc @shawnborton @tgolen noting once again that me re-opening the PR will only lose review context - it won't change the options, so my take is to move forward with Melvin in this PR regardless of selected option |
|
Thank you for that detailed breakdown, that is helpful! @tgolen any thoughts? Maybe we see if Melvin can do Option A above. @MelvinBot can you take a look at the latest comment and do Option A above, as well as fix conflicts? |
…andingAndSendToSomeone Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com> # Conflicts: # src/pages/iou/request/step/IOURequestStepDistance/handleMoneyRequestStepDistanceNavigation.ts
…lows + write-action guard - Wire isSelfDMDestination into the scan-skip, distance-skip and QAB amount cleanup call sites (and SubmitDetailsPage) so the LOOKING_AROUND route-to-Search gate actually fires on those paths, not only the confirmation step. - Guard getSkipConfirmationPreMountDestinationRoute so the self-DM isn't pre-inserted behind the RHP for LOOKING_AROUND users (no Personal Space flash before Search). - Drop the dead isLookingAroundUser flag (and its Onyx read) from the two invoice call sites where the self-DM can never be the destination. - Gate SEND_TO_SOMEONE on canUserPerformWriteAction so it hides on an archived self-DM. - Add unit coverage for the new skip-confirmation guard and the archived-self-DM case. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
Did Option A + resolved conflictsPushed ConflictsMerged Option A — the four mechanical items
Net effect: Bug 1 is now actually fixed on the scan-skip / distance-skip / QAB paths (the most common way a new user creates their first expense), not just the confirmation step — and the two invoice sites no longer carry a flag that can never fire. Tests
Honest caveats
I left the submit2026 two-destination split out (that's a coupled UX + copy decision, per the earlier thread). Let me know if you'd like me to pick that up too. |
|
Nice, let me know how that is feeling @ikevin127 @tgolen In the meantime, @MelvinBot can you try to fix the failing tests |
- IOURequestStepDistance is already at React Compiler's memoization-preservation limit; the extra top-level isLookingAroundUser const tipped it over and un-relaxed the manual-memo ESLint rule on two pre-existing useCallbacks. Compute the LOOKING_AROUND + self-DM flags inline at the call site so no new reactive const is added, restoring compilation (fixes both the React Compiler Compliance and ESLint checks). - spellcheck: reword 'misrouted' -> 'sent to Search by mistake' and fix the pre-existing 'overriden' -> 'overridden' typo surfaced in these changed files. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
|
Fixed the failing checks in
The only remaining red checks are the two human/process gates — One caveat on local verification
|
|
Taking a look at this now and I'll let you know what I think! |
tgolen
left a comment
There was a problem hiding this comment.
I think this PR is OK as-is for the scope of it and we can continue working on it to merge.
In the future, I would have split it into two PRs. One for "looking around users" and one for "send to someone".
| // Data needed to launch the track-expense "Choose a recipient" flow from the "Send to someone" row (mirrors the | ||
| // track-expense whisper in ChatActionableButtons). | ||
| const activePolicy = useActivePolicy(); | ||
| const {isRestrictedToPreferredPolicy, preferredPolicyID} = usePreferredPolicy(); | ||
| // Memoize the selector factory so useOnyx keeps a stable selector identity across this header's frequent re-renders | ||
| // (hold/violation/attribute churn) - an inline factory returns a fresh {filteredPoliciesCount, firstPolicyID} each render. | ||
| const filteredPoliciesInfoSelector = useMemo(() => createFilteredPoliciesInfoSelector(currentUserLogin), [currentUserLogin]); | ||
| const [filteredPoliciesInfo] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: filteredPoliciesInfoSelector}); | ||
| // Derive the draft IDs from the existing transactionDrafts subscription instead of subscribing to the same collection twice. | ||
| // validTransactionDraftsSelector is keyed by transactionID, so its keys are exactly the valid draft transaction IDs. | ||
| const draftTransactionIDs = useMemo(() => Object.keys(transactionDrafts ?? {}), [transactionDrafts]); |
There was a problem hiding this comment.
NAB: all of the comments here could probably be removed. They aren't super helpful because they just explain what the code is obviously doing already.
There was a problem hiding this comment.
Done — removed those comments in 0ee2434.
| // Reuse the exact track-expense whisper flow: create a draft from the tracked expense and open the | ||
| // "Choose a recipient" RHP. Scoped against the self-DM (parentReport), matching the whisper. | ||
| // Resolve the track-expense actionable whisper the same way the report-details menu does so the | ||
| // convert flow can mark the original self-DM whisper resolved once the expense is sent - passing | ||
| // undefined here would leave that whisper stranded and offering to submit an already-sent expense. |
There was a problem hiding this comment.
This comment isn't helpful either.
There was a problem hiding this comment.
Removed in 0ee2434.
| /** | ||
| * Returns true when the onboarding choice is "Something else" (LOOKING_AROUND). Extracted so callers can read the | ||
| * onboarding choice from Onyx in render context and pass the result into pure navigation helpers, rather than | ||
| * having those helpers subscribe to Onyx themselves. | ||
| */ | ||
| function isLookingAroundOnboardingChoice(choice: OnyxEntry<OnboardingPurpose>): boolean { | ||
| return choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND; | ||
| } |
There was a problem hiding this comment.
This function is not valuable and can be removed. Since it's just a single line of logic, replace the function with the logic directly wherever it appears.
There was a problem hiding this comment.
Done in 0ee2434 — removed isLookingAroundOnboardingChoice and inlined choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND at every call site.
|
Oops, I meant for that review to be "request changes" and clicked on the wrong button. Please address the comments :D |
|
I will remove my review request, but let me know if there is anything you want my take on 🙌 |
|
@MelvinBot can you please review the comments from Tim and address them? |
|
Looks like we're moving with option A with some caveats. All that's left from my side is to complete the PR Reviewer Checklist including running the manual tests. I'll do that shortly and will report back on how / what works ✅ |
…ay onboarding change - Remove the single-line isLookingAroundOnboardingChoice helper and inline 'choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND' at every call site. - Remove the unhelpful what-restating comments in MoneyRequestHeaderSecondaryActions. - Revert navigateAfterOnboarding.ts fully to main so it's out of this PR's diff — it had picked up an unrelated navigateToSubmitWorkspaceAfterOnboarding reroute (Spend to Workspace > Categories) that isn't part of this PR. Co-authored-by: Shawn Borton <shawnborton@users.noreply.github.com>
Addressed
|
| Tim's comment | What I did |
|---|---|
OnboardingUtils.ts — "this function isn't valuable, inline it" |
Removed isLookingAroundOnboardingChoice and inlined choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND at all 7 call sites. |
MoneyRequestHeaderSecondaryActions.tsx (×2) — "these comments aren't helpful" |
Removed the what-restating comments in both spots. |
navigateAfterOnboarding.ts — "get rid of this" |
Reverted the file fully to main so it's out of the diff. |
navigateAfterExpenseCreate.ts — "use useOnyx + params, not connectWithoutView" |
Already resolved in the prior round — it's a pure function taking isLookingAroundUser as a param; no connectWithoutView. |
One thing worth flagging: the reason navigateAfterOnboarding.ts was still in the diff wasn't the Looking-Around change (already reverted) — the file had picked up an unrelated change to navigateToSubmitWorkspaceAfterOnboarding that reroutes it from Spend › Expenses to Workspace › Categories. That isn't on main and isn't part of this PR, so I dropped it with the revert. If that reroute is actually wanted, it should go in its own PR.
Environment note
typecheck-tsgo is clean for the changed files; the only error is a missing jest-when dev-dependency in tests/unit/postOrReplaceComment.ts (a file I didn't touch), which resolves in CI where deps are installed. lint-changed and the React Compiler check-changed scripts can't compute a merge-base in this shallow checkout, so I ran ESLint directly on the changed files (passed); CI runs the full gate.
Review of
|
| Item | Status |
|---|---|
Pass isSelfDMDestination at the 3 real skip call sites |
✅ ScanSkipConfirmation (both cleanup calls), handleMoneyRequestStepDistanceNavigation (both), IOUAmountSubmission.submitSkipConfirmationExpense, all via isSelfDMSoleDestination(participants, iouType, currentUserAccountID). SubmitDetailsPage now passes isSelfDM(report) in cleanupParams too |
Drop isLookingAroundUser from the 2 invoice call sites |
✅ Gone from DynamicIOURequestStepCompanyInfo (file is out of the diff entirely, including the dead NVP_INTRO_SELECTED subscription) and from the invoice branch of useExpenseSubmission |
Guard getSkipConfirmationPreMountDestinationRoute |
✅ (isLookingAroundUser && isSelfDMDestination) added, mirroring getSubmitExpensePreMountDestinationRoute, wired into all three callers, plus 3 unit tests |
canUserPerformWriteAction on the SEND_TO_SOMEONE gate |
✅ Added alongside the split guard, with an archived-self-DM test case |
| Tests / Offline / QA sections filled | ✅ Written, with gaps noted in section 3 |
tgolen: inline isLookingAroundOnboardingChoice |
✅ Helper removed, inlined at all call sites |
tgolen: drop the what-restating comments |
✅ Removed |
Bug 1 now genuinely covers scan-skip, distance-skip and QAB, not just the confirmation step. Test count is 16 across 5 files. That part is done.
2. Findings
🔴 navigateAfterOnboarding.ts reverts a change that is currently on main — blocking
The claim in this comment and the inline reply is that the navigateToSubmitWorkspaceAfterOnboarding reroute "isn't on main" and was therefore dropped. That is backwards. gh pr diff diffs against the merge base, and this file shows 4 + / 9 -:
- const categoriesRoute = ROUTES.WORKSPACE_CATEGORIES.getRoute(policyID);
- const backToRoute = shouldUseNarrowLayout ? ROUTES.WORKSPACE_INITIAL.getRoute(policyID) : ROUTES.WORKSPACES_LIST.route;
- Navigation.navigate(`${categoriesRoute}?backTo=${encodeURIComponent(backToRoute)}` as Route);
-
+ Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.EXPENSE})}));Verified locally:
$ git show $(git merge-base HEAD origin/main):src/libs/navigateAfterOnboarding.ts | grep -c categoriesRoute
2
$ git show HEAD:src/libs/navigateAfterOnboarding.ts | grep -c categoriesRoute
0
$ git show origin/main:src/libs/navigateAfterOnboarding.ts | grep -c categoriesRoute
2
The Workspace › Categories routing has been on main since bd5daef6e46 (the reordered-onboarding-accounting-flow work). The "revert to main" took the file back to a pre-bd5daef state instead.
Bug this causes: a user who creates or joins a Submit workspace during onboarding currently lands on Workspace › Categories with the side panel open (the designed flow, with a layout-aware backTo). With this PR merged they land on Spend › Expenses instead, and the backTo is gone. That is an unrelated onboarding flow silently regressing inside a PR about Looking-Around routing, and nothing in this PR's test steps would catch it.
Fix: restore the file to the current merge-base state so it drops out of the diff entirely:
git checkout $(git merge-base HEAD origin/main) -- src/libs/navigateAfterOnboarding.ts
The buildCannedSearchQuery import and the Route type import go with it.
🟡 Two different predicates for isSelfDMDestination across the skip flows
The pre-mount guard only works if it suppresses in exactly the cases where the navigate half forces Search. Right now three screens do not agree on how that is computed:
| Screen | Pre-mount predicate | Navigate-half predicate |
|---|---|---|
ScanSkipConfirmation |
isSelfDMSoleDestination(participants, ...) |
isSelfDMSoleDestination(participants, ...) ✅ |
SubmitDetailsPage |
isSelfDM(report) |
isSelfDM(report) ✅ |
IOURequestStepAmount |
isSelfDM(report) |
isSelfDMSoleDestination(...) in IOUAmountSubmission |
IOURequestStepDistance |
isSelfDM(report) |
isSelfDMSoleDestination(...) in handleMoneyRequestStepDistanceNavigation |
Being straight about this: I traced the Amount and Distance paths and could not construct a case where the two actually disagree today, because when report is undefined the builder already bails on !reportID. So this is not a demonstrated bug. It is a latent one, and it is specifically the class of mismatch the guard exists to prevent, so it should not rely on !reportID covering for it. transaction?.participants is in scope in both screens (IOURequestStepAmount.tsx:127 already uses it), so both can use the same predicate as their navigate half:
const skipConfirmationPreMountRoute = getSkipConfirmationPreMountDestinationRoute(
shouldSkipConfirmation,
report?.reportID,
introSelected?.choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND,
isSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID),
);🟡 The new SEND_TO_SOMEONE gate tests leak spies into every later test in the file
tests/unit/ReportSecondaryActionUtilsTest.ts — getSendToSomeoneResult sets three spies and never restores them:
jest.spyOn(ReportUtils, 'isTrackExpenseReportNew').mockReturnValue(true);
jest.spyOn(ReportUtils, 'canUserPerformWriteAction').mockReturnValue(!isChatReportArchived);
jest.spyOn(TransactionUtils, 'getOriginalTransactionWithSplitInfo').mockReturnValue({...});The enclosing describe runs jest.clearAllMocks() in its beforeEach, which resets mock.calls but not mockReturnValue, and there is no restoreMocks in jest.config.js, jest/setupAfterEnv.ts, or the jest-expo preset. So every test declared after this block inside getSecondaryTransactionThreadActions runs with isTrackExpenseReportNew → true, canUserPerformWriteAction → false and a mocked getOriginalTransactionWithSplitInfo.
I ran the suite and it is green today (56 passed), so nothing is broken right now. But the next person adding a case below this block gets a silent false result. One line fixes it:
describe('SEND_TO_SOMEONE gate', () => {
afterEach(() => {
jest.restoreAllMocks();
});🟡 isLookingAroundUserSelector re-adds the one-liner that was just deleted
tgolen asked for isLookingAroundOnboardingChoice to be removed and inlined, which was done. But src/selectors/Onboarding.ts now adds:
function isLookingAroundUserSelector(introSelected: OnyxValue<typeof ONYXKEYS.NVP_INTRO_SELECTED>): boolean {
return introSelected?.choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND;
}I think this one is justified and should stay: a useOnyx selector needs a stable module-level identity, and it sits directly next to the identical isTrackIntentUserSelector, so it follows existing precedent rather than inventing a pattern. Flagging it only so it doesn't read as the same thing being re-added. @tgolen worth a quick confirm that the selector form is fine with you.
🟡 The PR's own Tests section uses the old label
Steps 3 and 6 of Bug 2 say "Submit it to someone", but the row now renders iou.sendToSomeone = "Send to someone". QA will look for a string that no longer exists.
3. Do the listed manual tests cover the diff ?
Mostly yes for Bug 1, and there are real gaps for Bug 2 and for the regression surface.
Well covered: the signup → Home → create → Spend loop across manual / scan-skip / distance, the narrow-layout no-flash check (this is the one that exercises the new getSkipConfirmationPreMountDestinationRoute guard), and the "user with a workspace still lands on their report" regression case, which is the guard that took three rounds to get right.
Missing, and worth adding before QA:
- Whisper resolution after "Send to someone." This is the fix for the most severe bug found in this PR and no step verifies it. Add: after step 5, go back to Personal Space and confirm the original "Submit it to someone / Categorize it / Share it" whisper is resolved and no longer offers actions on the sent expense.
- The split-expense gate.
!isSelfDMExpenseSplit || hasWorkspaceToSubmitTois new logic with no manual step. Two cases: self-DM split expense with no workspace → row hidden; same user after joining a workspace → row appears. - Invoice regression.
isLookingAroundUserwas removed from both invoice call sites. Add: as a Looking-Around user, create an invoice and confirm the landing is unchanged. - Submit-workspace onboarding. Because of finding 🔴 above, create or join a Submit workspace during onboarding and confirm you land on Workspace › Categories with the side panel open. This currently fails.
- Share flow.
SubmitDetailsPagechanged on both the pre-mount and cleanup halves; the Android/iOS share sheet → Submit details path has no step. - QAB split out from scan-skip. Step 3 folds "scan a receipt then skip confirmation (via the quick action button)" into one item, but those are two separate code paths (
ScanSkipConfirmationvsIOUAmountSubmission.submitSkipConfirmationExpensereached fromIOURequestStepAmount). Both changed, so both need their own step. - Delegate-restricted account. Tapping the row should show the no-access modal and not create a draft.
Offline section: step 2 is the right one. Step 3 ("row still appears offline") is trivially true and adds nothing. Replace it with the two that actually exercise the changed code: submit the "Send to someone" conversion while offline and confirm the whisper resolves optimistically; then force a failure and confirm the whisper rolls back to unresolved, since that rollback is gated on the same actionableWhisperReportActionID that was undefined two rounds ago.
@MelvinBot We need these addressed before proceeding with manual testing and merging.
Coming from here, understanding the caveats, and whether they affect issue #97881 expected resultIssue #97881 asks for exactly two things: land in Spend › Expenses after the first expense, and have "Send to someone" under More using the same flow as from the Inbox.
On that last one, concretely. With So under the beta the new row is not the same flow as the Inbox: it silently gives you the friend path only. Note: I don't think that blocks this PR. The row is labelled "Send to someone", so defaulting to the person destination is a defensible reading, and the split is a More-menu UX decision that wants Design input rather than a reviewer's guess. But it should be a conscious call rather than a deferral, so: @shawnborton is single-destination correct for this row under the Submit plan, or should it split like the whisper does? If single-destination is intended, let's say so in the PR description and close #97881 on it. If not, it's a follow-up issue and #97881 stays open on that half. |
- IOURequestStepAmount / IOURequestStepDistance: compute the skip-confirmation pre-mount guard with isSelfDMSoleDestination(transaction?.participants, ...), matching the navigate-half predicate instead of isSelfDM(report), so the guard suppresses in exactly the cases navigation forces Search (both files still pass the React Compiler compliance check). - ReportSecondaryActionUtilsTest: restore spies after each SEND_TO_SOMEONE-gate case so its mockReturnValue spies don't leak into later tests. Co-authored-by: Kevin Brian Bader <ikevin127@users.noreply.github.com>
|
Thanks for the thorough pass, 🔴
|
| Commit | Date (UTC) | Message |
|---|---|---|
8df2536 |
2026-08-14 15:33 | Land new Submit workspace users on Spend > Expenses |
c275146 |
2026-08-13 17:22 | Route new Submit workspace users to Home instead of Categories |
c275146 removed the WORKSPACE_CATEGORIES + backTo routing you quoted, and 8df2536 set it to SEARCH_ROOT/buildCannedSearchQuery — i.e. main itself now lands new Submit-workspace users on Spend › Expenses. The branch is byte-identical to that:
$ git diff origin/main HEAD -- src/libs/navigateAfterOnboarding.ts
(empty)
$ git show origin/main:src/libs/navigateAfterOnboarding.ts | grep -c categoriesRoute
0
$ git show HEAD:src/libs/navigateAfterOnboarding.ts | grep -c categoriesRoute
0
So merging this PR changes that file by zero lines relative to current main, and there's no Submit-workspace onboarding regression. Applying git checkout <merge-base> -- navigateAfterOnboarding.ts would do the opposite of what's wanted now — it would re-introduce the pre-c275146 Categories routing that main deliberately removed yesterday. I've left the file untouched (it already matches main). If you re-verify, please git fetch first — the difference is entirely main moving under the earlier review.
🟡 Cleanups — done in e26deb5
- Unified the self-DM predicate.
IOURequestStepAmountandIOURequestStepDistancenow compute the pre-mount guard withisSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID)— the same predicate family as their navigate half — instead ofisSelfDM(report), so it no longer leans on!reportIDto paper over a mismatch. Both files still passreact-compiler-compliance-check(kept inline in Distance to stay under its memo-preservation limit). - Test spy leak. Added
afterEach(() => jest.restoreAllMocks())to theSEND_TO_SOMEONE gateblock inReportSecondaryActionUtilsTestso itsmockReturnValuespies stop leaking into latergetSecondaryTransactionThreadActionscases. isLookingAroundUserSelector— agreed, leaving it as-is (stableuseOnyxselector identity, mirrorsisTrackIntentUserSelector).tgolena quick 👍 on the selector form when you get a sec.- PR Tests label. Fixed
Submit it to someone→Send to someonethroughout the description (the only remaining "Submit it to someone" is the intentional reference to the whisper copy in the whisper-resolution step).
🟡 Manual test steps — added
Rewrote the description's Tests / Offline / QA sections to cover the gaps you listed: whisper resolution after Send to someone, the split-expense gate (hidden without a workspace, shown with one), archived self-DM, delegate-restricted, invoice regression, submit-workspace onboarding regression, the Share flow, and QAB split out from scan-skip as its own step. Offline now has the optimistic-resolve + rollback-on-failure pair instead of the trivial "row still shows offline" step.
One correction on your suggested submit-workspace step: the expected landing is Spend › Expenses, not Workspace › Categories — per c275146/8df2536 above that's what current main does, and this PR doesn't touch that path, so it's framed as a "matches main" regression check.
Verification (Android)
Confirmed Bug 2 on standalone Android NewDot — created a self-tracked expense, opened its More menu:
Send to someone renders first with a divider beneath it, and tapping it opens the Choose recipient RHP.
More menu — "Send to someone" first, divider beneath
Choose recipient screen after tapping "Send to someone"
npm run fmt, ESLint, typecheck-tsgo, react-compiler-compliance-check (both step files), and the unit suites pass locally. (One local typecheck-tsgo error in tests/unit/postOrReplaceComment.ts is an env-only missing jest-when install — it's declared in package.json and isn't part of this PR, so CI is unaffected.)
|
🟢 Everything looks good code-wise - I'm moving to manual testing. |
|
🧪 I ran the manual tests and found some issues that require fixes: Setup
GATE 1 — Looking-Around landing, all four create paths (wide layout)
|
Local patch: Gates 1, 2, 3 and 6I applied a fix locally and re-ran the merge gates. All six now pass, including Gate 2 on narrow, which I'd previously flagged as out of scope. It turned out to be fixable without touching shared behaviour, so no follow-up is needed on this PR. DIFFdiff --git a/src/libs/IOUAmountSubmission.ts b/src/libs/IOUAmountSubmission.ts
index 15a873e4a7d..4bfc4fa0b78 100644
--- a/src/libs/IOUAmountSubmission.ts
+++ b/src/libs/IOUAmountSubmission.ts
@@ -436,6 +436,9 @@ function submitSkipConfirmationExpense(args: SubmitAmountArgs, ctx: SubmitAmount
submitWithDismissFirst({
executeWrite: executeExpenseWrite,
destinationReportID: isTrackExpenseSubmit ? (report?.reportID ?? selfDMReport?.reportID) : report?.reportID,
+ isFromGlobalCreate: getIsFromGlobalCreate(transaction),
+ isLookingAroundUser: introSelected?.choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND,
+ isSelfDMDestination,
telemetryContext: {
scenario: isTrackExpenseSubmit ? CONST.TELEMETRY.SUBMIT_EXPENSE_SCENARIO.TRACK_EXPENSE : CONST.TELEMETRY.SUBMIT_EXPENSE_SCENARIO.REQUEST_MONEY_MANUAL,
iouType,
diff --git a/src/libs/Navigation/helpers/navigateAfterExpenseCreate.ts b/src/libs/Navigation/helpers/navigateAfterExpenseCreate.ts
index 60218e882ad..05b151c1b96 100644
--- a/src/libs/Navigation/helpers/navigateAfterExpenseCreate.ts
+++ b/src/libs/Navigation/helpers/navigateAfterExpenseCreate.ts
@@ -114,7 +114,11 @@ function navigateAfterExpenseCreate({
} else if (getIsNarrowLayout()) {
const isRHPStillOnTop = navigationRef.getRootState()?.routes?.at(-1)?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR;
if (!alreadyOnSearchRoot || !isSameSearchType || isRHPStillOnTop) {
- Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: queryString}), {forceReplace: true});
+ // forceReplace makes linkTo dispatch a REPLACE against TAB_NAVIGATOR, but SEARCH.ROOT is in linkTo's
+ // ROOT_TAB_SCREENS, so the cross-tab PUSH branch is skipped and the REPLACE resolves to a no-op, leaving
+ // the user on the tab they submitted from. Skipped only for the LOOKING_AROUND self-DM flow so every
+ // other caller keeps its existing history behaviour.
+ Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: queryString}), {forceReplace: !(isLookingAroundUser && isSelfDMDestination)});
} else {
Log.info('[IOU] navigateToSearch: already on matching Search root with RHP dismissed - no-op');
}
diff --git a/src/libs/Navigation/helpers/submitWithDismissFirst.ts b/src/libs/Navigation/helpers/submitWithDismissFirst.ts
index de854b12f26..4ed2455cc95 100644
--- a/src/libs/Navigation/helpers/submitWithDismissFirst.ts
+++ b/src/libs/Navigation/helpers/submitWithDismissFirst.ts
@@ -24,6 +24,12 @@ type SubmitWithDismissFirstParams = {
destinationReportID: string | undefined;
/** Telemetry metadata for the submit-expense performance span. */
telemetryContext: SubmitExpenseContext;
+ /** Whether the expense was initiated from the global FAB (no pre-existing report context). */
+ isFromGlobalCreate?: boolean;
+ /** Whether the user onboarded as "Something else" (LOOKING_AROUND) - they have no workspace. */
+ isLookingAroundUser?: boolean;
+ /** Whether the sole destination for this expense is the current user's self-DM (Personal Space). */
+ isSelfDMDestination?: boolean;
};
function startDismissFirstTracking(
@@ -47,14 +53,22 @@ function startDismissFirstTracking(
*
* 1. Search topmost -> dismiss modal, defer write for Search skeleton
* 2. Route pre-inserted -> dismiss modal, write after transition (route already staged)
- * 3. Destination already shown -> dismiss modal, write after transition
- * 4. Destination loaded -> reveal destination then dismiss, write after transition
- * 5. Destination not loaded -> write immediately, then reveal-and-dismiss
- * 6. Fallback -> start tracking with default fast path, write with defaults
+ * 3. Looking-Around self-DM -> write owns navigation (routes to Search via navigateAfterExpenseCreate)
+ * 4. Destination already shown -> dismiss modal, write after transition
+ * 5. Destination loaded -> reveal destination then dismiss, write after transition
+ * 6. Destination not loaded -> write immediately, then reveal-and-dismiss
+ * 7. Fallback -> start tracking with default fast path, write with defaults
*
* Must not be called from `src/libs/actions/` — view-layer only.
*/
-function submitWithDismissFirst({executeWrite, destinationReportID, telemetryContext}: SubmitWithDismissFirstParams): void {
+function submitWithDismissFirst({
+ executeWrite,
+ destinationReportID,
+ telemetryContext,
+ isFromGlobalCreate = false,
+ isLookingAroundUser = false,
+ isSelfDMDestination = false,
+}: SubmitWithDismissFirstParams): void {
const shouldStayOnSearch = isSearchTopmostFullScreenRoute();
if (shouldStayOnSearch) {
@@ -75,6 +89,18 @@ function submitWithDismissFirst({executeWrite, destinationReportID, telemetryCon
return;
}
+ // "Something else" (LOOKING_AROUND) users have no workspace, so a global-create expense lands in their self-DM, but
+ // the product requirement is to route them to Spend > Expenses instead. Every dismiss-first branch below calls
+ // executeWrite with shouldHandleNavigation: false, and cleanupAfterSkipConfirmSubmit drops the routing flags in that
+ // case - so navigateAfterExpenseCreate never runs and the destinationReportID branch reveals the self-DM. Hand
+ // navigation back to the write, same as the fallback below, so navigateAfterExpenseCreate owns the Search routing.
+ if (isFromGlobalCreate && isLookingAroundUser && isSelfDMDestination) {
+ startTracking(telemetryContext, {skipSubmitExpenseSpan: true});
+ setFastPath(CONST.TELEMETRY.FAST_PATH_HANDLER.DEFAULT);
+ executeWrite({shouldHandleNavigation: true});
+ return;
+ }
+
if (destinationReportID) {
const isDestinationLoaded = !!getReportOrDraftReport(destinationReportID)?.reportID;
const isNarrow = getIsNarrowLayout();
diff --git a/src/pages/iou/request/step/IOURequestStepAmount.tsx b/src/pages/iou/request/step/IOURequestStepAmount.tsx
index 596c6cee24a..3da031edd08 100644
--- a/src/pages/iou/request/step/IOURequestStepAmount.tsx
+++ b/src/pages/iou/request/step/IOURequestStepAmount.tsx
@@ -21,7 +21,7 @@ import Log from '@libs/Log';
import {getAmountHasUnsavedChanges} from '@libs/MoneyRequestUtils';
import Navigation from '@libs/Navigation/Navigation';
import {getParticipantsOption, getReportOption} from '@libs/OptionsListUtils';
-import {getTransactionDetails, isMoneyRequestReport, isPolicyExpenseChat, shouldEnableNegative} from '@libs/ReportUtils';
+import {getTransactionDetails, isMoneyRequestReport, isPolicyExpenseChat, isSelfDM, shouldEnableNegative} from '@libs/ReportUtils';
import {getRequestType, isDistanceRequest, isExpenseUnreported} from '@libs/TransactionUtils';
import MoneyRequestAmountForm from '@pages/iou/MoneyRequestAmountForm';
@@ -159,13 +159,16 @@ function IOURequestStepAmount({
return !(isReportArchived || isPolicyExpenseChat(report));
}, [report, isSplitBill, skipConfirmation, isReportArchived]);
- // Use the same self-DM predicate as the navigate half (IOUAmountSubmission) so the pre-mount guard suppresses in exactly
- // the cases where navigation forces Search, rather than relying on report?.reportID being undefined to cover a mismatch.
+ // Both self-DM signals are ORed on purpose. The navigate half (IOUAmountSubmission) reads participants at submit time,
+ // but on a quick-action flow they are not populated yet when this pre-mount decision runs, so the participants check
+ // alone misses and the self-DM gets pre-inserted - then navigateAfterExpenseCreate reveals it instead of going to Search.
+ // isSelfDM(report) answers the question this site actually cares about ("is the report I am about to pre-insert the
+ // self-DM?") and is available immediately.
const skipConfirmationPreMountRoute = getSkipConfirmationPreMountDestinationRoute(
shouldSkipConfirmation,
report?.reportID,
isLookingAroundUser,
- isSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID),
+ isSelfDM(report) || isSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID),
);
usePreMountDestination(skipConfirmationPreMountRoute);
diff --git a/src/pages/iou/request/step/IOURequestStepDistance.tsx b/src/pages/iou/request/step/IOURequestStepDistance.tsx
index c8f98e35874..6ff5b44b283 100644
--- a/src/pages/iou/request/step/IOURequestStepDistance.tsx
+++ b/src/pages/iou/request/step/IOURequestStepDistance.tsx
@@ -41,7 +41,7 @@ import Navigation from '@libs/Navigation/Navigation';
import OnyxTabNavigator, {TabScreenWithFocusTrapWrapper, TopTab} from '@libs/Navigation/OnyxTabNavigator';
import {roundToTwoDecimalPlaces} from '@libs/NumberUtils';
import {isTrackOnboardingChoice} from '@libs/OnboardingUtils';
-import {isPolicyExpenseChat as isPolicyExpenseChatUtil} from '@libs/ReportUtils';
+import {isPolicyExpenseChat as isPolicyExpenseChatUtil, isSelfDM} from '@libs/ReportUtils';
import {getDistanceInMeters, getRateID, getRequestType, getSelectedRouteKey, hasManualDistanceOverride, haveWaypointAddressesChanged} from '@libs/TransactionUtils';
import CONST from '@src/CONST';
@@ -321,7 +321,9 @@ function IOURequestStepDistance({
introSelected?.choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND,
// Same self-DM predicate as the navigate half (handleMoneyRequestStepDistanceNavigation) so the guard suppresses in
// exactly the cases navigation forces Search. Kept inline to stay under this component's React Compiler memo limit.
- isSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID),
+ // Both self-DM signals are ORed: on a quick-action flow participants are not populated yet when this runs, so the
+ // participants check alone misses and the self-DM gets pre-inserted, which navigateAfterExpenseCreate then reveals.
+ isSelfDM(report) || isSelfDMSoleDestination(transaction?.participants ?? [], iouType, currentUserPersonalDetails.accountID),
);
usePreMountDestination(skipConfirmationPreMountRoute);
diff --git a/src/pages/iou/request/step/IOURequestStepDistance/handleMoneyRequestStepDistanceNavigation.ts b/src/pages/iou/request/step/IOURequestStepDistance/handleMoneyRequestStepDistanceNavigation.ts
index ccc2596400b..8dcf697f64b 100644
--- a/src/pages/iou/request/step/IOURequestStepDistance/handleMoneyRequestStepDistanceNavigation.ts
+++ b/src/pages/iou/request/step/IOURequestStepDistance/handleMoneyRequestStepDistanceNavigation.ts
@@ -279,6 +279,9 @@ function handleMoneyRequestStepDistanceNavigation({
if (isCreatingTrackExpense && participant) {
submitWithDismissFirst({
+ isFromGlobalCreate: transactionIsFromGlobalCreate,
+ isLookingAroundUser,
+ isSelfDMDestination,
// trackExpense is a void action with no navigation params; submitWithDismissFirst owns dismiss/reveal and cleanup runs after.
executeWrite: (overrides) => {
trackExpense({
@@ -366,6 +369,9 @@ function handleMoneyRequestStepDistanceNavigation({
const distanceDestinationReportID = report?.reportID;
submitWithDismissFirst({
+ isFromGlobalCreate: transactionIsFromGlobalCreate,
+ isLookingAroundUser,
+ isSelfDMDestination,
executeWrite: (overrides) => {
const {transactionID: writtenDistanceTransactionID} = createDistanceRequest({
report,
diff --git a/src/pages/iou/request/step/IOURequestStepScan/components/ScanSkipConfirmation.tsx b/src/pages/iou/request/step/IOURequestStepScan/components/ScanSkipConfirmation.tsx
index c8813b5077a..a0b894cc11c 100644
--- a/src/pages/iou/request/step/IOURequestStepScan/components/ScanSkipConfirmation.tsx
+++ b/src/pages/iou/request/step/IOURequestStepScan/components/ScanSkipConfirmation.tsx
@@ -323,6 +323,9 @@ function ScanSkipConfirmation({report, action, iouType, reportID, transactionID,
const scanDestinationReportID = iouType === CONST.IOU.TYPE.TRACK ? (report?.reportID ?? selfDMReport?.reportID) : report?.reportID;
submitWithDismissFirst({
+ isFromGlobalCreate,
+ isLookingAroundUser,
+ isSelfDMDestination,
executeWrite: (overrides) => {
// Cleanup runs after each write (not once up front) so a stalled GPS lookup can't clear the draft before the expense exists.
const runCleanup = () =>
diff --git a/src/pages/iou/request/step/confirmation/SubmitExpenseOrchestrator.tsx b/src/pages/iou/request/step/confirmation/SubmitExpenseOrchestrator.tsx
index ab4ea05e1a9..c681ccedf29 100644
--- a/src/pages/iou/request/step/confirmation/SubmitExpenseOrchestrator.tsx
+++ b/src/pages/iou/request/step/confirmation/SubmitExpenseOrchestrator.tsx
@@ -284,6 +284,12 @@ function SubmitExpenseOrchestrator({
// When Search is not visible (e.g. submitting from Home/Settings), we must navigate there.
const isSearchVisible = isSearchTopmostFullScreenRoute();
const shouldNavigateToSearch = !isSameType || !isSearchVisible;
+ // forceReplace makes linkTo dispatch a REPLACE against TAB_NAVIGATOR, but SEARCH.ROOT is in linkTo's
+ // ROOT_TAB_SCREENS, so the cross-tab PUSH branch is skipped and the REPLACE resolves to a no-op: the mounted
+ // tab navigator keeps its state and the user stays on the tab they submitted from. Skipping forceReplace makes
+ // the navigation actually happen. Scoped to the LOOKING_AROUND self-DM flow this fix is about - other callers
+ // keep forceReplace so their browser-history behaviour is unchanged, even though they hit the same no-op today.
+ const shouldSkipForceReplace = isFromGlobalCreateForNavigation && isLookingAroundUser && isSelfDMDestination;
setPendingSubmitFollowUpAction(shouldNavigateToSearch ? CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.NAVIGATE_TO_SEARCH : CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.DISMISS_MODAL_ONLY);
reserveDeferredWriteChannel(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH);
@@ -321,7 +327,7 @@ function SubmitExpenseOrchestrator({
return;
}
- Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: buildCannedSearchQuery({type: searchType})}), {forceReplace: true});
+ Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: buildCannedSearchQuery({type: searchType})}), {forceReplace: !shouldSkipForceReplace});
});
},
});
diff --git a/tests/unit/navigateAfterExpenseCreateTest.ts b/tests/unit/navigateAfterExpenseCreateTest.ts
index c17fecc6ddf..26ec395ea9a 100644
--- a/tests/unit/navigateAfterExpenseCreateTest.ts
+++ b/tests/unit/navigateAfterExpenseCreateTest.ts
@@ -112,10 +112,30 @@ describe('navigateAfterExpenseCreate', () => {
isSelfDMDestination: true,
});
- expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_ROOT.getRoute({query: 'type:expense'}), {forceReplace: true});
+ // forceReplace is deliberately false here: it makes linkTo dispatch a REPLACE against TAB_NAVIGATOR, and because
+ // SEARCH.ROOT is a tab root that REPLACE is a no-op, which left these users stuck on the tab they submitted from.
+ expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_ROOT.getRoute({query: 'type:expense'}), {forceReplace: false});
expect(Navigation.dismissModalWithReport).not.toHaveBeenCalled();
});
+ it('should keep forceReplace for a narrow Search navigation that is not the LOOKING_AROUND self-DM flow', () => {
+ // The forceReplace opt-out is scoped to the flow this fix is about, so every other caller keeps its existing
+ // browser-history behaviour even though they hit the same linkTo no-op today.
+ mockIsReportTopmostSplitNavigator.mockReturnValue(false);
+ mockGetIsNarrowLayout.mockReturnValue(true);
+
+ navigateAfterExpenseCreate({
+ activeReportID: 'report-123',
+ transactionID: 'txn-1',
+ isFromGlobalCreate: true,
+ hasMultipleTransactions: false,
+ isLookingAroundUser: false,
+ isSelfDMDestination: false,
+ });
+
+ expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.SEARCH_ROOT.getRoute({query: 'type:expense'}), {forceReplace: true});
+ });
+
it('should NOT route a LOOKING_AROUND user to search when the destination is a real report (not the self-DM)', () => {
// A LOOKING_AROUND user who later has a workspace and submits to a real report/friend from the Inbox must open that
// report, not be permanently misrouted to Search. isSelfDMDestination is false, so they are treated as "on inbox".
diff --git a/tests/unit/submitWithDismissFirstTest.ts b/tests/unit/submitWithDismissFirstTest.ts
index e826ea351ed..e3fc66ddfd6 100644
--- a/tests/unit/submitWithDismissFirstTest.ts
+++ b/tests/unit/submitWithDismissFirstTest.ts
@@ -194,6 +194,106 @@ describe('submitWithDismissFirst', () => {
});
});
+ describe('Looking-Around self-DM branch', () => {
+ const LOOKING_AROUND_SELF_DM = {
+ isFromGlobalCreate: true,
+ isLookingAroundUser: true,
+ isSelfDMDestination: true,
+ };
+
+ it('hands navigation to the write instead of revealing the self-DM report', () => {
+ // Without this branch the destination-report fast path reveals the self-DM and calls executeWrite with
+ // shouldHandleNavigation: false, which makes cleanupAfterSkipConfirmSubmit drop the routing flags before
+ // navigateAfterExpenseCreate can route these users to Spend > Expenses.
+ mockGetReportOrDraftReport.mockReturnValue({reportID: 'selfDM1'});
+ const executeWrite = jest.fn();
+
+ submitWithDismissFirst({
+ executeWrite,
+ destinationReportID: 'selfDM1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ });
+
+ expect(executeWrite).toHaveBeenCalledWith({shouldHandleNavigation: true});
+ expect(mockRevealRouteBeforeDismissingModal).not.toHaveBeenCalled();
+ expect(mockDismissModal).not.toHaveBeenCalled();
+ });
+
+ it('still starts tracking so telemetry is not skipped', () => {
+ mockGetReportOrDraftReport.mockReturnValue({reportID: 'selfDM1'});
+
+ submitWithDismissFirst({
+ executeWrite: jest.fn(),
+ destinationReportID: 'selfDM1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ });
+
+ expect(mockStartTracking).toHaveBeenCalledWith(TELEMETRY_CONTEXT, {skipSubmitExpenseSpan: true});
+ expect(mockSetFastPath).toHaveBeenCalledWith(CONST.TELEMETRY.FAST_PATH_HANDLER.DEFAULT);
+ });
+
+ it('does not divert when the destination is not the self-DM', () => {
+ mockGetReportOrDraftReport.mockReturnValue({reportID: 'r1'});
+ const executeWrite = jest.fn();
+
+ submitWithDismissFirst({
+ executeWrite,
+ destinationReportID: 'r1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ isSelfDMDestination: false,
+ });
+
+ expect(mockRevealRouteBeforeDismissingModal).toHaveBeenCalled();
+ expect(executeWrite).not.toHaveBeenCalledWith({shouldHandleNavigation: true});
+ });
+
+ it('does not divert when the expense is not from global create', () => {
+ mockGetReportOrDraftReport.mockReturnValue({reportID: 'selfDM1'});
+
+ submitWithDismissFirst({
+ executeWrite: jest.fn(),
+ destinationReportID: 'selfDM1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ isFromGlobalCreate: false,
+ });
+
+ expect(mockRevealRouteBeforeDismissingModal).toHaveBeenCalled();
+ });
+
+ it('does not divert for a non-Looking-Around user', () => {
+ mockGetReportOrDraftReport.mockReturnValue({reportID: 'selfDM1'});
+
+ submitWithDismissFirst({
+ executeWrite: jest.fn(),
+ destinationReportID: 'selfDM1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ isLookingAroundUser: false,
+ });
+
+ expect(mockRevealRouteBeforeDismissingModal).toHaveBeenCalled();
+ });
+
+ it('yields to the Search-topmost branch, which already leaves the user on Search', () => {
+ mockIsSearchTopmostFullScreenRoute.mockReturnValue(true);
+ const executeWrite = jest.fn();
+
+ submitWithDismissFirst({
+ executeWrite,
+ destinationReportID: 'selfDM1',
+ telemetryContext: TELEMETRY_CONTEXT,
+ ...LOOKING_AROUND_SELF_DM,
+ });
+
+ expect(mockDismissModal).toHaveBeenCalledTimes(1);
+ expect(executeWrite).not.toHaveBeenCalled();
+ });
+ });
+
describe('Priority', () => {
it('Search-topmost takes priority over destination report', () => {
mockIsSearchTopmostFullScreenRoute.mockReturnValue(true);10 files, +187 / −14. Fix 1 — the skip-confirmation flags never reached the navigator (Gates 1, 3, 6)
function cleanupAfterSkipConfirmSubmit(shouldHandleNavigation: boolean, params: CleanupAndNavigateAfterExpenseCreateParams) {
if (shouldHandleNavigation) {
cleanupAndNavigateAfterExpenseCreate(params);
return;
}
// isLookingAroundUser and isSelfDMDestination are dropped here
cleanupAfterExpenseCreate({draftTransactionIDs: params.draftTransactionIDs, linkedTrackedExpenseReportAction: params.linkedTrackedExpenseReportAction});
}And Navigation.revealRouteBeforeDismissingModal(ROUTES.REPORT_WITH_ID.getRoute(destinationReportID));which reveals and dismisses to the self-DM. Same root cause as the 2026-08-07 fix, one layer over. That round correctly taught The change: a new branch in Threaded at four call sites, all of which already had the values in scope: Fix 2 — narrow layout (Gate 2)Two separate causes, both confirmed with console instrumentation on narrow desktop Chrome and iOS mWeb Safari. 2a.
|
| Change | Gate |
|---|---|
submitWithDismissFirst new branch |
isFromGlobalCreate && isLookingAroundUser && isSelfDMDestination |
Orchestrator forceReplace opt-out |
isFromGlobalCreateForNavigation && isLookingAroundUser && isSelfDMDestination |
navigateAfterExpenseCreate forceReplace opt-out |
!(isLookingAroundUser && isSelfDMDestination), and that line is only reachable past the !isFromGlobalCreate early return |
isSelfDM(report) || isSelfDMSoleDestination(...) in Amount / Distance |
Indirect: the only consumer is getSkipConfirmationPreMountDestinationRoute, which uses it exclusively inside (isLookingAroundUser && isSelfDMDestination), so for other users the widened value is ANDed against false and discarded |
For any non-LOOKING_AROUND user, and for any LOOKING_AROUND user whose expense does not land in the self-DM, behaviour is bit-identical to main — forceReplace: true is preserved, the branch order in submitWithDismissFirst is unchanged, and pre-insert eligibility is unchanged. The callers I deliberately left alone (PAY / send-money, scan split) omit the params entirely and take the defaults.
Pinned by tests: navigateAfterExpenseCreateTest now asserts forceReplace: false for the LOOKING_AROUND self-DM case and forceReplace: true for a narrow Search navigation that is not it, so a future refactor cannot quietly widen the opt-out.
Deliberately not fixed here
The underlying linkTo bug is real and untouched: forceReplace no-ops for any tab-root target, so other narrow flows that reach SEARCH_DISMISS (e.g. a workspace user creating an expense from Home, via getSubmitHandler's last branch) are likely affected on staging and production today. Fixing that properly belongs in linkTo so forceReplace is honoured for tab roots, which would touch every caller. That is out of scope here and worth its own issue. Scoping our fix to this flow means we neither depend on it nor regress it.
Verification
- 261 tests pass across the 6 related suites. New: 6 cases in
submitWithDismissFirstTest(branch fires; yields to Search-topmost; three negatives for non-self-DM destination, non-global-create, non-Looking-Around) and 1 innavigateAfterExpenseCreateTestpinning theforceReplacescoping. One existing assertion innavigateAfterExpenseCreateTestwas updated fromforceReplace: truetofalse, since that is the fix. typecheck-tsgoclean,npm run fmtapplied, React Compiler compliance passes onScanSkipConfirmation,IOURequestStepAmount,IOURequestStepDistanceandSubmitExpenseOrchestrator.- ESLint could not run in my environment (
eslint-plugin-storybookthrowsERR_REQUIRE_ESMon Node 20.13, unrelated to the change), so please rely on CI for that one.
Gate results after the patch
| Gate | Result |
|---|---|
| 1 - all four create paths, wide | ✅ pass |
| 2 - narrow layout (incl. quick action, scan-skip, distance-skip) | ✅ pass |
| 3 - workspace user not misrouted | ✅ pass (run from Inbox with the chat loaded) |
| 4 - Send to someone row + whisper resolution | ✅ pass |
| 5 - row hides when it should | ✅ pass |
| 6 - offline | ✅ pass |
@MelvinBot please apply the changes above using the diff as the guide.
…rch + fix narrow forceReplace - Add a LOOKING_AROUND self-DM branch to submitWithDismissFirst that hands navigation back to the write so navigateAfterExpenseCreate owns the Search routing (QAB, scan-skip, distance-skip). Threaded from IOUAmountSubmission, ScanSkipConfirmation and both handleMoneyRequestStepDistanceNavigation call sites; PAY/SPLIT callers left untouched. - Skip forceReplace for the LOOKING_AROUND self-DM flow in navigateAfterExpenseCreate's narrow branch and SubmitExpenseOrchestrator.handleSearchDismiss (a REPLACE against a tab-root Search no-ops in linkTo). Every other caller keeps forceReplace: true. - OR isSelfDM(report) into the pre-mount guard's self-DM signal in IOURequestStepAmount and IOURequestStepDistance so the quick-action self-DM isn't pre-inserted then revealed. - Tests: new submitWithDismissFirst LOOKING_AROUND describe block; pin forceReplace scoping in navigateAfterExpenseCreateTest (false for the flow, true otherwise). Co-authored-by: ikevin127 <39289794+ikevin127@users.noreply.github.com> Co-authored-by: Kevin Brian Bader <ikevin127@users.noreply.github.com>
|
Applied your patch verbatim in What landed
Verification I ran
One caveat: I couldn't add a live web pass on top of your manual verification — the prestarted web session was handed off in a blank, unresponsive state (empty accessibility tree, no interactive surface) that didn't recover, so there was nothing to drive. Since your patch already carries the six-gate manual results and the change is gated to the LOOKING_AROUND self-DM flow, I'm relying on that plus the green unit suites here. Please still lean on CI for the full lint/typecheck/test run. |
Reviewer Checklist
Screenshots/VideosiOS: HybridApp
iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.17.Pro.Max.-.2026-08-14.at.21.25.52.movMacOS: Chrome / Safariweb.mov |
|
@Expensify/design 🟢 Completed the PR Reviewer Checklist: this looks good to me and seems complete. Now before we merge it I'd want design to actually trigger an adhoc build and test the issue-expected flows on both web and mobile layouts and confirm that the issue's expected result was achieved. |




Explanation of Change
Fixes two issues a brand-new "Something else" (
LOOKING_AROUND) user hits after creating their first expense.Bug 1 — new user lands in Personal Space instead of Spend › Expenses.
A
LOOKING_AROUNDuser has no report or workspace, so after onboarding they land on the Home page (Inbox). When they then create an expense from Home, that expense lands in their self-DM, and the app was dropping them into that self-DM (Personal Space) instead of Spend › Expenses.Fix: when a
LOOKING_AROUNDuser creates an expense whose sole destination is their self-DM, route them to Spend › Expenses (Search) instead of the self-DM. This is scoped toisSelfDMDestination, so aLOOKING_AROUNDuser who later has a workspace and submits to a real report/friend keeps their normal destination. The routing decision is threaded through all three post-create decision points that must agree with each other —getSubmitExpensePreMountDestinationRoute(what is pre-mounted behind the RHP),getSubmitHandler(dismiss strategy), andnavigateAfterExpenseCreate(final landing) — and through every create path: the confirmation step and the skip-confirmation flows (scan-and-skip, distance-skip, and quick-amount entry). The self-DM is never pre-inserted behind the RHP for these users, so there's no Personal Space flash before the switch to Search on narrow layout. Onboarding landing (navigateAfterOnboarding) is unchanged — these users still land on Home after signup.Bug 2 — "Send to someone" missing from a tracked expense's More menu.
When viewing an unreported self-tracked expense (Spend › View Expense), the More menu had no way to send it to someone, even though the track-expense actionable whisper offers exactly that from the Inbox.
Fix: add a Send to someone row at the top of the expense header's More menu (
MoneyRequestHeaderSecondaryActions), with a divider beneath it, reusing the whisper's existingcreateDraftTransactionAndNavigateToParticipantSelectorhelper — opening the identical "Choose a recipient" RHP with zero changes to the picker. The row is gated so it only appears for an unreported self-tracked expense in personal space where the convert-from-track flow applies (requires write access, so it hides on an archived self-DM; and hides for a self-DM split with no workspace to submit to); once the expense is submitted to a workspace/report the gate is false and the row hides. It uses a dedicatediou.sendToSomeonetranslation key ("Send to someone") across all locales.Fixed Issues
$ #97881
PROPOSAL:
Tests
Bug 1 — Looking-Around user lands in Spend › Expenses after creating an expense
ScanSkipConfirmation), and verify you still land on Spend › Expenses.IOUAmountSubmission), and verify you still land on Spend › Expenses.main(the invoice paths no longer carry the Looking-Around flag).main(this PR does not modifynavigateAfterOnboardingrelative to currentmain).Bug 2 — "Send to someone" on a tracked expense's More menu
Offline tests
reportActionIDthe conversion now resolves.QA Steps
Same as the Tests section above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari